Extend get guestbooks use case with stats and Download responses#449
Extend get guestbooks use case with stats and Download responses#449ChengShi-1 wants to merge 10 commits into
Conversation
There was a problem hiding this comment.
Pull request overview
Adds optional guestbook usage/response statistics support to the “get guestbooks by collection” flow, aligning the client with the Dataverse API enhancement needed for the Manage Guestbooks UI.
Changes:
- Extended
getGuestbooksByCollectionIdto accept an optionalincludeStatsflag and send it as a query parameter when enabled. - Updated the
Guestbookdomain model to optionally exposeusageCountandresponseCount. - Added/updated unit + integration tests, plus documentation and changelog entries.
Reviewed changes
Copilot reviewed 9 out of 9 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| test/unit/guestbooks/GuestbooksRepository.test.ts | New unit coverage for repository query param behavior and error handling. |
| test/unit/guestbooks/GetGuestbooksByCollectionId.test.ts | Adds unit coverage for the use case passing includeStats. |
| test/integration/guestbooks/GuestbooksRepository.test.ts | Adds integration coverage for listing stats and verifying count changes via assignment + guest response submission. |
| src/guestbooks/infra/repositories/GuestbooksRepository.ts | Adds includeStats param and forwards it as query params when true. |
| src/guestbooks/domain/useCases/GetGuestbooksByCollectionId.ts | Extends use case API to accept includeStats. |
| src/guestbooks/domain/repositories/IGuestbooksRepository.ts | Updates repository interface to include optional includeStats. |
| src/guestbooks/domain/models/Guestbook.ts | Adds optional usageCount and responseCount fields. |
| docs/useCases.md | Documents the new includeStats parameter and updates example usage. |
| CHANGELOG.md | Adds an Unreleased entry describing the new includeStats support. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
ekraffmiller
left a comment
There was a problem hiding this comment.
Hi @ChengShi-1 I left some comments on the PR, hope it's not too confusing! After going back on forth between the use cases and the API, I think there is actually one more change we need in the API - which I describe in my comment in GetGuestbookResponsesOfAGuestbook. I'm happy to discuss more about it.
| /** | ||
| * Returns guestbook responses for one guestbook in a dataverse collection. | ||
| * | ||
| * @param {number | string} dataverseId - Dataverse identifier. |
There was a problem hiding this comment.
The API for getting the responses for a guestbook in JSON form is, for example:
curl -H "X-Dataverse-key:$API_TOKEN" "$SERVER_URL/api/guestbooks/$ID/responses?limit10&offset=0"
So we need to add parameters for limit and offset.
Another thing I noticed though, is that this API doesn't allow us to pass a dataverseId, so it's going to return all guestbook responses regardless of collection. So unfortunately I think we to ask for another change to the API - to be able to pass guestbookId and collectionId and get all the responses for a guestbook within a specific collection, returned in JSON format.
| import { GuestbookResponse } from '../models/GuestbookResponse' | ||
| import { IGuestbooksRepository } from '../repositories/IGuestbooksRepository' | ||
|
|
||
| export class GetGuestbookResponsesByDataverseId implements UseCase<GuestbookResponse[]> { |
There was a problem hiding this comment.
I don't think this use case is needed, we only need to download the csv version of the responses, we don't need an array of JSON for displaying.
What this PR does / why we need it:
includeStatsandincludeInheritedsupport togetGuestbooksByCollectionId.Which issue(s) this PR closes:
Related Dataverse PRs:
Special notes for your reviewer:
Test failed about storage drive, so I also copy pastes a
CollectionHelper.tsfrom this PR. waiting to be mergedhttps://github.com/IQSS/dataverse-client-javascript/pull/431/changes#diff-edac108c1a43d5773116d19797d04ecdf890fc97b5cf83ce2d56d38b9a7aca18
Suggestions on how to test this:
Is there a release notes or changelog update needed for this change?:
Additional documentation: